Skip to content

Swift: extract still unextracted entities from the 6.0.2 upgrade #19299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Apr 15, 2025

Conversation

redsun82
Copy link
Contributor

@redsun82 redsun82 commented Apr 14, 2025

Tests were not added when a timeboxed effort to make Swift produce the new entity did not bear any fruit.

@github-actions github-actions bot added the Swift label Apr 14, 2025
@redsun82 redsun82 marked this pull request as ready for review April 14, 2025 12:45
@Copilot Copilot bot review requested due to automatic review settings April 14, 2025 12:45
@redsun82 redsun82 requested a review from a team as a code owner April 14, 2025 12:45
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements support for new Swift entities introduced in version 6.0.2 by adding translation functions and updating tag mappings.

  • Added two new expression translation functions: one for extracting function isolation and one for retrieving current context isolation.
  • Updated tag mappings in SwiftTagTraits to replace placeholder void values with explicit tags.

Reviewed Changes

Copilot reviewed 24 out of 41 changed files in this pull request and generated no comments.

File Description
swift/extractor/translators/ExprTranslator.h Declares new translation functions for handling isolation expressions.
swift/extractor/translators/ExprTranslator.cpp Implements the new translation functions with appropriate label fetching.
swift/extractor/infra/SwiftTagTraits.h Updates mappings for ActorIsolationErasureExpr, UnreachableExpr, ExtractFunctionIsolationExpr, and CurrentContextIsolationExpr using explicit tags.
Files not reviewed (17)
  • swift/downgrades/be2357fd0023261478871eff5df5c57df559aa3b/downgrade.ql: Language not supported
  • swift/downgrades/be2357fd0023261478871eff5df5c57df559aa3b/upgrade.properties: Language not supported
  • swift/ql/.generated.list: Language not supported
  • swift/ql/.gitattributes: Language not supported
  • swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll: Language not supported
  • swift/ql/lib/codeql/swift/elements.qll: Language not supported
  • swift/ql/lib/codeql/swift/elements/expr/ActorIsolationErasureExpr.qll: Language not supported
  • swift/ql/lib/codeql/swift/elements/expr/CurrentContextIsolationExpr.qll: Language not supported
  • swift/ql/lib/codeql/swift/elements/expr/ExtractFunctionIsolationExpr.qll: Language not supported
  • swift/ql/lib/codeql/swift/elements/expr/UnreachableExpr.qll: Language not supported
  • swift/ql/lib/codeql/swift/elements/expr/internal/ActorIsolationErasureExprConstructor.qll: Language not supported
  • swift/ql/lib/codeql/swift/elements/expr/internal/ActorIsolationErasureExprImpl.qll: Language not supported
  • swift/ql/lib/codeql/swift/elements/expr/internal/CurrentContextIsolationExprConstructor.qll: Language not supported
  • swift/ql/lib/codeql/swift/elements/expr/internal/CurrentContextIsolationExprImpl.qll: Language not supported
  • swift/ql/lib/codeql/swift/elements/expr/internal/ExtractFunctionIsolationExprConstructor.qll: Language not supported
  • swift/ql/lib/codeql/swift/elements/expr/internal/ExtractFunctionIsolationExprImpl.qll: Language not supported
  • swift/ql/lib/codeql/swift/elements/expr/internal/UnreachableExprConstructor.qll: Language not supported
Comments suppressed due to low confidence (2)

swift/extractor/translators/ExprTranslator.cpp:674

  • The newly introduced translation functions for ExtractFunctionIsolationExpr and CurrentContextIsolationExpr do not have accompanying tests. Consider adding tests to verify that the correct labels are being fetched and that the entry objects are populated as expected.
codeql::ExtractFunctionIsolationExpr ExprTranslator::translateExtractFunctionIsolationExpr(const swift::ExtractFunctionIsolationExpr& expr) {

swift/extractor/infra/SwiftTagTraits.h:205

  • With the updated mappings replacing the TODO placeholders, it would be beneficial to add tests ensuring that these tag values are correctly utilized during the extraction process.
MAP(swift::ExtractFunctionIsolationExpr, ExtractFunctionIsolationExprTag)

@jketema
Copy link
Contributor

jketema commented Apr 14, 2025

Looks like the DCA experiment failed?

@redsun82
Copy link
Contributor Author

Looks like the DCA experiment failed?

I think it's a DCA hiccup, I had a similar thing happenign on a Rust DCA. Tried the retry.

Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, assuming DCA comes back happy.

@redsun82
Copy link
Contributor Author

There was a -3 successfully extracted files on the first DCA run in the Dovve repo, but a rerun on that same repo has shown a +3 there, which indicates that number is flaky.

@redsun82 redsun82 merged commit 3d3de78 into main Apr 15, 2025
21 checks passed
@redsun82 redsun82 deleted the redsun82/swift-6-extraction branch April 15, 2025 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants